perf(gemma4): an admission coalesce door protects the decode tail - #1013
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 543c062d15
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let door_open = match state.admit_coalesce { | ||
| Some(window) | ||
| if !pending.is_empty() | ||
| && !active.is_empty() | ||
| && (active.len() + pending.len()) * 2 >= state.slots => |
There was a problem hiding this comment.
When both PEGAINFER_ADMIT_COALESCE_MS and PEGAINFER_ASYNC_PREFILL are enabled, this condition still closes the door, although admit_from_queue sends only one request to the async lane and immediately stops once that lane is busy. The waiting requests therefore never share the mixed admission that coalescing promises; they only incur repeated coalescing delays between serialized lane launches. Either reject this knob combination or bypass the door whenever the async lane is configured.
AGENTS.md reference: AGENTS.md:L48-L48
Useful? React with 👍 / 👎.
| /// The admission coalesce door: `PEGAINFER_ADMIT_COALESCE_MS=N` holds | ||
| /// arrivals that would ride a live decode batch for up to `N` ms, so one | ||
| /// window's arrivals share a single mixed admission instead of invading | ||
| /// the batch once each — a live stream's tail gap prices the number of | ||
| /// admission events, not their size. An idle engine admits on sight; the | ||
| /// door only prices arrivals that have someone to disturb. Unset (or | ||
| /// `0`/`off`) admits on sight everywhere. | ||
| fn admit_coalesce_ms() -> Result<Option<std::time::Duration>> { |
There was a problem hiding this comment.
This introduces a public environment variable with nontrivial range, default, and traffic-dependent behavior, but the commit leaves both the root Key env vars list and docs/models/gemma4/serving.md unchanged. Operators consequently cannot discover the knob or its interactions from the repository's serving documentation, contrary to the required workflow to update relevant documents while implementing a change.
AGENTS.md reference: AGENTS.md:L174-L178
Useful? React with 👍 / 👎.
c72ab7c to
17b43f6
Compare
|
the Issue and PR now say a window's arrivals land as one capacity-bounded back-to-back intake burst, name the 512-row gather budget explicitly, and state that with 1024-token prompts every admission is its own mixed step. No architectural change is attempted. The title becomes The timeout language now follows your three cases: a full cohort releases before N, the timeout release lands no earlier than N at the first intake turn after the window elapses, and there is no hard bound on how much later, since the engine drains its submission channel before each intake. The old door-plus-lane lifecycle line is removed as well — that combination now refuses at load, before a device opens — and the verification describes the gates as they run today, with the serving-knob guard clearing the door everywhere and the dedicated intake gate as the one place it is enabled. That gate passes at the exact head through the maintainer runner: a closed wait schedules nothing while decode advances the incumbent, the cohort release drains one burst with one Scheduled event per waiter, the timeout release lands at the injected deadline, and a full roster admits nothing until a slot frees. |
Signed-off-by: Feathbow <feathbow@gmail.com>
admit_coalesce_ms hand-rolled std::env::var and spelled its variable three times, and SERVING_KNOBS omitted it, so the test-env guard neither cleared nor restored the door. The name becomes a const beside its siblings, the getter reads through read_env, and the guard covers all six knobs. Signed-off-by: Feathbow <feathbow@gmail.com>
…ombine The lane flies one prefill at a time and admission stops while it is busy, so a door in front of it can only delay serialized launches. The combination now refuses in EngineState::load before a device opens, and the early-refusal gate covers it. Signed-off-by: Feathbow <feathbow@gmail.com>
The door block moves onto CoalesceDoor with an injected clock: the depth gate, the capacity-bounded cohort, the early release, the window timeout and the clears are unit-tested, and a checkpoint gate drives a live roster through the closed arm - two arrivals wait, the third releases the burst, one intake pass admits the cohort. The contract note moves onto the type and states what the door buys: a burst of admissions, not a shared weight scan. Signed-off-by: Feathbow <feathbow@gmail.com>
The knob joins the Key env vars list and the serving doc gains its section: what the door prices, the gather-budget caveat, the lane refusal, and the measured trade-off that keeps it off by default. Signed-off-by: Feathbow <feathbow@gmail.com>
…te share The dedicated gate drove a privately built door and called admission itself, so deleting the door conditional, never constructing the door, or admitting through a closed door would all have passed. The door construction and the decision-plus-conditional-admission move onto EngineState (coalesce_door, intake_turn); the loop calls them, and the gate drives the same functions with an injected clock through four scenarios: a closed wait that schedules nothing while decode advances the incumbent, a cohort release draining one burst with one Scheduled event per waiter, a timeout release, and a full roster admitting nothing until a slot frees. Signed-off-by: Feathbow <feathbow@gmail.com>
The serving doc states the release lands at the first intake after the window elapses (checked once per engine iteration, so the wait can exceed N by up to one decode round), and the index row for the serving doc gains the knob. Signed-off-by: Feathbow <feathbow@gmail.com>
17b43f6 to
7c96bdb
Compare
1 participant
Description
Closes #1011
PEGAINFER_ADMIT_COALESCE_MS=Nholds arrivals that would ride a live decode batch for up to N ms, so one window's arrivals share a single mixed admission instead of invading the batch once each — a live stream's tail gap prices the number of admission events, not their size. A full mixed cohort releases early, a shallow batch (live rows plus arrivals under half the slots) skips the door, and an idle engine admits on sight. Unset,0oroffserves byte-identically to before; the door is off by default.It stays off by default on the numbers below: under closed-loop load the c8 tail under the door is bistable — the completion waves either synchronize into cohorts and the tail drops to the other engine's band, or they do not and the door only charges first-token time — c4 gains nothing, and c16 gains. Open-loop Poisson arrivals, the measurement that decides the serving default, show the closed-loop tail to be a bench artefact of synchronized fixed-length cohorts; the door is a profile for traffic shaped like that.
Test Env
Single GPU (
sm_89, 48 GiB, x86_64)Verification
cargo fmt --all -- --checkandgit diff --checkagainst the base pass; the Gemma 4 release server builds; all-target Gemma Clippy passes with-D warnings; release lib tests pass (43, the door's value parser among them).Unset: the 26B checkpoint answers four greedy public
/v1/completionsrequests (31-, 35-, 33- and 1421-token prompts, 64 forced tokens) with bytes identical tomain's binary, md5219de1e3c09c; the idle-refill, full-roster, roster-edge, mixed-step and gathered-walk gates pass.PEGAINFER_ADMIT_COALESCE_MS=5000and=abcrefuse to start with the parser's message; with the door at 300 ms the shared-lane and gathered lifecycles, the roster edge and the full-roster gate pass.Serving A/B on one binary, door off against
PEGAINFER_ADMIT_COALESCE_MS=300,vllm bench serverandom 1024 in / 128 out at 24 / 48 / 64 prompts, two cooled alternating rounds, mean [range]:c4 is flat — the shallow-batch skip keeps the door out. Both c8 rounds landed on the coin's tax side: the completion waves did not synchronize into cohorts, so the door charged 5.7% throughput and 19 ms of first-token time for no tail change; the campaign round that caught the cohort attractor (ITL p99 falling from the high band to the other engine's) does so with roughly even odds, which is exactly why the door is not the default. c16 trades first-token time (+288 ms median) for the decode tail's pace (median TPOT −8.5%) at flat throughput.